-
Notifications
You must be signed in to change notification settings - Fork 106
[v12] feat(ui-form-field): rework FormFieldGroup #2311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
| label: 'formFieldGroup', | ||
| border: `${componentTheme.borderWidth} ${componentTheme.borderStyle} ${componentTheme.borderColor}`, | ||
| borderRadius: componentTheme.borderRadius, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these tokens were used for the old error style (border around all the children, newError style does not use them), but I forgot to remove them form here: #2273
| const generateStyle = ( | ||
| componentTheme: FormFieldGroupTheme, | ||
| _componentTheme: FormFieldGroupTheme, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both FormFieldGroup and FormFieldMessages are components that have no tokens anymore and therefore the style.ts files have no theme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the FormFieldGroupTheme type, its not used? As I see in withStyle its always an empty object, so it can be like this:
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
_componentTheme: {},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it like that above
4549eee to
e2711a5
Compare
| const generateStyle = ( | ||
| componentTheme: FormFieldGroupTheme, | ||
| _componentTheme: FormFieldGroupTheme, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the FormFieldGroupTheme type, its not used? As I see in withStyle its always an empty object, so it can be like this:
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
_componentTheme: {},
INSTUI-4871
e2711a5 to
a7242d7
Compare
joyenjoyer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
INSTUI-4871
ISSUE:
TEST PLAN: